462254 - Remove EULA from Firefox installer / .dmg and related bits

Status: VERIFIED FIXED

(Firefox :: Installer, defect)

Description

[Justin Dolske [:Dolske]]

+++ This bug was initially created as a clone of Bug #456439 +++

Bug 456439 adds a new "about:rights" page and notification bar. The intention is for that to also replace the current EULA stuff that's shown to Windows and OS X users when installing Firefox. [The about:rights notification bar isn't shown if the user was already shown the EULA.]

IIRC, there are also some trivial scattered "EULA" references that we may want to take care of at the same time.

Comment 1

[Justin Dolske [:Dolske]]

Attachment: [Mac] Patch v.1

This removes the changes the OS X .dmg to no longer prompt for a EULA agreement, and removes the licenses from the tree. The code to build a dmg with a license is still there (through we don't use it), maybe Mozilla-derived products will want to use it?

Untested, doing builds now. :-)

Comment 2

[Justin Dolske [:Dolske]]

Ok, verified that I can build a .dmg with a EULA prompt, and that with this patch that no longer happens.

One problem, though. :( The pref removal from firefox.js makes sense for new installs, but will cause existing profiles to be shown the about:rights notification again -- argh. Not sure what the best way is to avoid this. Maybe check something like browser.startup.homepage_override.mstone and set the pref to not show the notification? If there's not a good solution, I wonder how bad it would be to just show the notification to all existing users. :/

Comment 3

[Justin Dolske [:Dolske]]

Attachment: [Win] Patch v.1

Comment 4

[Justin Dolske [:Dolske]]

Attachment: [all] Patch v.1 (fix for existing profiles)

This fixes the problem noted in comment 2.

The specific goal is to set browser.rights.3.shown to true when a OS X or Windows user upgrades across a boundary with the other patches in this bug. [Because we changed the default pref, but don't want to nag users who already saw the EULA in the old installer.] The broader-but-simpler patch is to just always set browser.rights.3.shown when the browser sees that the user is upgrading an existing profile. In other words, having |savedmstone| set means you've got an existing profile, and there's no case where we want to show the about:rights notification in an existing profile. [I could move this outside the |if (mstone != savedmstone)|, but figured this avoids potential Ts impact at the cost of nagging nightly users when we check this in.]

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 346348 [details] [diff] [review]
[all] Patch v.1 (fix for existing profiles)

I'm tempted to say that you should just put this outside the mstone!=savedmstone check... It's very unlikely to make a difference for Ts, and it means we'll get immediate nightly testing of this.

Comment 6

[Justin Dolske [:Dolske]]

Pushed changeset e05886ad61db.

Comment 7

[Samuel Sidler (old account; do not CC)]

Dolske: fyi (same as in bug 456439), this bug blocks 1.9.0.5, which freezes in one week on November 17. When you have an appropriate 1.9.0 branch patch, please request approval1.9.0.5 on it.

Comment 8

[Samuel Sidler (old account; do not CC)]

Oh, it'd also be good to get QA verification of this bug on all relevant platforms before we take it on the 1.9.0 branch.

Comment 9

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081110 Minefield/3.1b2pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081110 Minefield/3.1b2pre.  EULA in the installer is gone, and about:rights exists in the notification bar on new profiles.

Comment 10

[Tony Chung [:tchung]]

flagging in-litmus? to add a testcase for the removal of eula in the installer.  I believe we already have a testcase for the notification bar.

Comment 11

[Tony Chung [:tchung]]

can we get this automated please?
Litmus test added: https://litmus.mozilla.org/show_test.cgi?id=7414.

Comment 12

[Justin Dolske [:Dolske]]

I don't think there's a good way of automating this, because most of the change is in the installer/dmg, and the rest of the code only happens on first run in a new profile (this would probably need a new top-level test framework).

Comment 13

[Justin Dolske [:Dolske]]

Attachment: [Mac] Branch patch v.1

Same changes as trunk, currently doing a build running to verify.

Comment 14

[Justin Dolske [:Dolske]]

Attachment: [Win] Branch patch v.1

Also same as trunk patch, currently building to verify. CVS doesn't seem to realize that browser/EULA.rtf was removed, so the patch doesn't show that.

Comment 15

[Robert Strong (they/them - no direct email)]

Comment on attachment 348672 [details] [diff] [review]
[Win] Branch patch v.1

Not sure what the approach you are planning to use for the l10n changes but we usually just leave the extra strings for branches.

Comment 16

[Justin Dolske [:Dolske]]

Comment on attachment 348666 [details] [diff] [review]
[Mac] Branch patch v.1

a? for the [Mac], [Win], and [All] (same as trunk) patches. Shouldn't affect anything outside the installer, same changes as landed on trunk earlier.

l10n shouldn't be an issue, not adding any strings here. [The l10n folks are aware of the strings bug 456439 adds to implement about:rights, if that's what you're asking.]

Comment 17

[Robert Strong (they/them - no direct email)]

(In reply to comment #16)
>...
> l10n shouldn't be an issue, not adding any strings here. [The l10n folks are
> aware of the strings bug 456439 adds to implement about:rights, if that's what
> you're asking.]
I'm not positive (it has been a while since I looked) but I thought the l10n tinderbox reported extra strings.

Comment 18

[Justin Dolske [:Dolske]]

Oh. Hmm. Axel, is that the case? If so, I'll just land the patch with those strings left in place. [Apparently the freeze is slushy again, so landing tomorrow should be fine.]

Comment 19

[Axel Hecht [:Pike]]

Yes, please leave the obsolete strings in CVS.

Comment 20

[Daniel Veditz [:dveditz]]

Comment on attachment 348672 [details] [diff] [review]
[Win] Branch patch v.1

Approved for 1.9.0.5, a=dveditz for release-drivers  -- but please leave the strings in as Axel requested.

Comment 21

[Daniel Veditz [:dveditz]]

Comment on attachment 348666 [details] [diff] [review]
[Mac] Branch patch v.1

Approved for 1.9.0.5, a=dveditz for release-drivers

Comment 22

[Justin Dolske [:Dolske]]

Attachment: Branch patch v.1 (unified), as checked in.

Checked in on FF3.0 branch:

Removing browser/EULA.rtf;
  new revision: delete; previous revision: 1.4
Checking in browser/app/Makefile.in;
  new revision: 1.160; previous revision: 1.159
Removing browser/app/macbuild/license.r;
  new revision: delete; previous revision: 1.4
Checking in browser/app/profile/firefox.js;
  new revision: 1.341; previous revision: 1.340
Checking in browser/branding/unofficial/Makefile.in;
  new revision: 1.7; previous revision: 1.6
Removing browser/branding/unofficial/license.r;
  new revision: delete; previous revision: 1.4
Checking in browser/components/nsBrowserContentHandler.js;
  new revision: 1.50; previous revision: 1.49
Checking in browser/installer/Makefile.in;
  new revision: 1.31; previous revision: 1.30
Checking in browser/installer/windows/Makefile.in;
  new revision: 1.44; previous revision: 1.43
Checking in browser/installer/windows/nsis/installer.nsi;
  new revision: 1.46; previous revision: 1.45
Checking in browser/locales/Makefile.in;
  new revision: 1.67; previous revision: 1.66
Checking in other-licenses/branding/firefox/Makefile.in;
  new revision: 1.27; previous revision: 1.26
Removing other-licenses/branding/firefox/license.r;
  new revision: delete; previous revision: 1.4
Checking in toolkit/mozapps/installer/windows/nsis/common.nsh;
  new revision: 1.44; previous revision: 1.43

Comment 23

[Justin Dolske [:Dolske]]

(Strings mentioned in comments 17-19 were left in the tree. Built an installer with them in place to verify that it didn't cause a problem.)

Comment 24

[Justin Dolske [:Dolske]]

Note about attachment 346348 [details] [diff] [review]:

That patch prevents the problem described in comment 2 and comment 4 (showing the about:rights notification to existing users who already accepted the EULA). The implicit assumption is that an existing profile must have already accepted the old EULA. This makes sense for our builds (which displayed the EULA on install or first-run)... But if the builds in linux distros were suppressing the old EULA, it raises the question if a further fix is needed (in the distro builds) to ensure that their existing users (who have not seen the old EULA) are shown the about:rights notification.

Discussed this with beltzner, and decided this was OK as-is. Users with an existing Firefox profile don't need to be shown the about:rights notification, even if they might not have been presented the old EULA.

New profiles will, of course, be shown the notification. Also, should we bump the about:rights version in the future, doing so would result in showing the about:rights notification to all users (new and existing, linux distro or not).

Comment 25

[Al Billings [:abillings - ex-MoCo]]

Verified that this is fixed in 1.9.0.5 with Mac, Windows, and Linux builds. None of them get the EULA anymore and they get the "know your rights" infobar with new profiles.

Comment 26

[Carsten Book [:Tomcat]]

Note: for a Major Update from 2.0.0.20 to 3.0.5 you get a License Text (see Bug 470258). You need to accept this license to do the Major Update.

The Problem is that you don't get the "know your rights" infobar after the MU...

Comment 27

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #26)
> The Problem is that you don't get the "know your rights" infobar after the
> MU...

That was intentional, existing users don't get the infobar since they've already accepted one of the EULAs.

Comment 28

[:Gavin Sharp [email: [e-mail]]]

Though I guess in the major update case they'd have seen only the 2.0 EULA. We can probably fix the check to only skip it for users running a previous 3.0.x version, or just fix bug 470258 such that the about:rights text is shown in the major update offer.